Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Maintaining trailing zeros for decimal precision in json output #1497

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Maintaining trailing zeros for decimal precision in json output #1497

wants to merge 2 commits into from

Conversation

florianschoffke
Copy link

Description:
This PR attempts to address the issue of trailing zeros for decimal numbers which are omitted when using Typescript and JSON tooling.
The main idea for solving this issue are twofold:

  1. In case of decimals the rawValue as string is being used for assignment
  2. When outputting the JSON content a regex replaces Numbers with trailing zeros that are strings to exclude the strings

There was no 'elegant' way that I could see to solve this issue as numbers in Typescript with trailing zeros are represented as plain numbers and fs.outputJSONsync transforms the number 19.00 to 19 as well.

Would be interested if we could work with that solution. If I missed other side effects please let me know.

Testing Instructions:
Run Sushi with this FSH File:

Instance: ObservationExample
InstanceOf: Observation
Usage: #example
* id = "example-xxx"
* extension[+]
  * url = "decimal-test-round"
  * valueDecimal = 19.00
* extension[+]
  * url = "decimal-test-off"
  * valueDecimal = 19.01
* extension[+]
  * url = "money-test-round"
  * valueMoney.value = 19.00
* extension[+]
  * url = "money-test-off"
  * valueMoney.value = 19.01
* status = #final
* code = http://loinc.org#11557-6

Before the commit the output would be "valueDecimal": 19
Afterwards it is "valueDecimal": 19.00

Related Issue:
#1215

@mint-thompson
Copy link
Collaborator

Unfortunately, I think this implementation has some undesirable side effects. Any string which can be turned into a number will end up as a number in the output JSON, regardless of the FHIR field's type. If you can check the FHIR field's type before making this conversion, and only change it when it is an appropriate type, then it would be good. I'm not sure how feasible it is to make that check during writeFHIRResources, though, since that is very late in the pipeline.

This FSH demonstrates a few cases where a string looks enough like a number that it gets turned into a number. The output JSON will have numeric values for all three valueString elements.

Instance: ObservationExample
InstanceOf: Observation
Usage: #example
* id = "example-xxx"
* extension[+]
  * url = "leading-zero-string"
  * valueString = "003413"
* extension[+]
  * url = "exponential-string"
  * valueString = "12E9"
* extension[+]
  * url = "hexadecimal-string"
  * valueString = "0X3A"
* status = #final
* code = http://loinc.org#11557-6

@cmoesel
Copy link
Member

cmoesel commented Aug 2, 2024

Hi @florianschoffke. Thanks again for the contribution! We really appreciate it! Preserving trailing 0s is definitely a pain point that JavaScript/JSON does not make easy. It would be great to have a solution for this. As Mint said, however, we need to be sure we don't break anything else in the process. Have you had a chance to think about the issues that Mint has raised? Do you plan to attempt to address them?

@florianschoffke
Copy link
Author

First off thanks for considering!
As of now I can't be sure that I will be able to solve it in the near future. But will have a look at the issues raised.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jafeltra jafeltra Awaiting requested review from jafeltra

@cmoesel cmoesel Awaiting requested review from cmoesel

@mint-thompson mint-thompson Awaiting requested review from mint-thompson

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@florianschoffke @mint-thompson @cmoesel